Skip to content

feat: return job IDs from Worker::perform_later() for status tracking#1624

Open
NewtTheWolf wants to merge 19 commits intoloco-rs:masterfrom
NewtTheWolf:feature/expose-worker-job-ids
Open

feat: return job IDs from Worker::perform_later() for status tracking#1624
NewtTheWolf wants to merge 19 commits intoloco-rs:masterfrom
NewtTheWolf:feature/expose-worker-job-ids

Conversation

@NewtTheWolf
Copy link
Contributor

Summary

This PR implements job ID returns from Worker::perform_later() to enable job status tracking, addressing #1623.

Changes

  • Modified Queue::enqueue() to return Result<Option<String>> instead of Result<()>
  • Modified BackgroundWorker::perform_later() to return Result<Option<String>> instead of Result<()>
  • All queue providers (Redis, PostgreSQL, SQLite) now return their internally tracked job IDs
  • Returns Some(job_id) when using BackgroundQueue mode with a configured provider
  • Returns None for ForegroundBlocking, BackgroundAsync modes, or BackgroundQueue without a provider

Testing

  • Updated all existing enqueue tests to verify job ID returns
  • Added validation in tests to verify that returned job IDs are valid ULIDs
  • Added new test queue_enqueue_returns_job_id to verify behavior for different queue modes
  • All bgworker tests pass successfully with cargo test --features bg_redis,bg_pg,bg_sqlt

Documentation

  • Updated API documentation for Queue::enqueue() method
  • Updated user documentation in docs-site/content/docs/processing/workers.md
  • Added usage example showing how to use the returned job ID
  • Added changelog entry

Notes for Reviewers

  • This is a non-breaking change - existing code that ignores the return value continues to work
  • The implementation follows the approach suggested in the original issue discussion

Implements #1623

@NewtTheWolf
Copy link
Contributor Author

i did not changed anything in the files who failed sanity 🤔

@kaplanelad
Copy link
Contributor

thanks @NewtTheWolf! .
Can you take a look at the conflicts?

@NewtTheWolf
Copy link
Contributor Author

yes, i can take a look this week

@kaplanelad
Copy link
Contributor

Hey @NewtTheWolf,
I tested this and I think there’s an issue.

Steps to Reproduce

  1. Create a clean project:

    ❯ loco new
    ✔ ❯ App name? · myapp
    ✔ ❯ What would you like to build? · Saas App with server side rendering
    ✔ ❯ Select a DB Provider · Sqlite
    ✔ ❯ Select your background worker type · Async (in-process tokio async tasks)
    
  2. Generate a worker:

    cargo loco generate worker test
    
  3. Generate a test controller:

    cargo loco generate controller test --api
    
  4. Add the following code to the controller:

    #[debug_handler]
    pub async fn index(State(ctx): State<AppContext>) -> Result<Response> {
     let job: Option<String> =
         workers::test::Worker::perform_later(&ctx, workers::test::WorkerArgs {}).await?;
     println!("job: {job:#?}",);
    
     format::empty()
    }
    
  5. Run the server with the worker:

    cargo loco start --server-and-worker
    
  6. Open the endpoint in your browser: http://localhost:5150/api/tests

Expected
I should see a job ID in the logs.

Actual i got: job: None

@NewtTheWolf
Copy link
Contributor Author

hey @kaplanelad thats an expected result

a Job ID ONLY gets returned if the Worker Supports it, if it does not Support JobId's it returns None

I saw you used Async Worker and Async Worker dont Return or even Generate Job ID's if i'm right?

i used this comment as Inspiration #1039 (comment)
Espacaliy:

NOTE: if the Redis provider cannot supply a job ID, then we can return a Result, and providers which do not support this return None

@kaplanelad
Copy link
Contributor

a Job ID ONLY gets returned if the Worker Supports it, if it does not Suppor

The intention is that we can switch queues behind the scenes and the app continues to work. Adding an optional Job ID could break logic when switching between different job queues.

Wouldn’t it make sense to generate a Job ID for all queues to avoid this issue?

@NewtTheWolf
Copy link
Contributor Author

it would make sense absoloutly. i generally like the idea of #1624 (comment)

sorry i really didnt had time for this, my goal is to update this pr this week!

@mccormickt
Copy link
Contributor

@NewtTheWolf do you need any help getting this across? This is quite a helpful feature that I'd love to get in. Let me know if you'd like any assistance.

@NewtTheWolf
Copy link
Contributor Author

hey @mccormickt, sorry had a loot going on in my live lately. But your Message was a small boost to my Motivation! i will start now! first of all making the Tokio Worker to return an ID so it is no longer Optional and then making the fn get_jobs public!

@jtwaleson
Copy link
Contributor

@NewtTheWolf I'd also appreciate this feature :)

@NewtTheWolf
Copy link
Contributor Author

@NewtTheWolf I'd also appreciate this feature :)

omg thats completly my fault. i thought it is finished, i just realize now that one CI has an error.... i will fix it tomorrow (UTC+1)

@jtwaleson
Copy link
Contributor

omg thats completly my fault. i thought it is finished, i just realize now that one CI has an error.... i will fix it tomorrow (UTC+1)

the best timezone 😂

No worries, looking forward to using it.

marlon-costa-dc pushed a commit to marlon-costa-dc/loco that referenced this pull request Feb 25, 2026
…s#1624)

- Queue::enqueue() now returns Result<Option<String>> with job ID
- BackgroundWorker::perform_later() returns job ID for tracking
- Queue::get_jobs made public for external job querying
- Exposes tokio task ID in BackgroundAsync worker mode
@NewtTheWolf
Copy link
Contributor Author

FINALLY

Copy link
Contributor

@mccormickt mccormickt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for hanging in there and getting this across the line.
cc @kaplanelad

@kaplanelad
Copy link
Contributor

Thanks @NewtTheWolf

A few things I'd like us to address before merging:

Inconsistent ID semantics across worker modes

The main goal here is status tracking, but the three modes return very different kinds of IDs:

  • BackgroundQueue — returns a real ULID from the queue provider, queryable via get_jobs(). This is the intended use case and works great.
  • ForegroundBlocking — returns a fresh Ulid::new() that is never persisted. Not queryable, but the job is already done by the time the ID is returned, so this is arguably fine.
  • BackgroundAsync — returns a tokio::task::Id (an integer like "42"), not a ULID and not in any queue. This is the most concerning case — if a user switches from BackgroundQueue to BackgroundAsync, their code that passes the returned ID to get_jobs() will silently fail. The format is also completely different (integer vs ULID).

Could we at least generate a ULID for BackgroundAsync too, so the return value has a consistent format across all modes? Even if it's not queryable, it avoids breaking code that parses/validates the ID format.

Error on missing provider (mod.rs lines 651-654)

Previously, if BackgroundQueue mode was configured but no provider was set, it logged an error and continued. Now it returns Err(...). This will crash apps that previously worked in a degraded state. Could we keep this as a warning/error log, or at minimum call this out as an intentional behavior change?

ulid as mandatory dependency

Since it's now used in all modes (not just behind bg_* feature flags), ulid is pulled in for every Loco user. This is a small crate so probably fine, but worth noting.

@NewtTheWolf
Copy link
Contributor Author

@kaplanelad

i will take a look into it.

BackgroundAsync — returns a tokio::task::Id (an integer like "42"), not a ULID and not in any queue. This is the most concerning case — if a user switches from BackgroundQueue to BackgroundAsync, their code that passes the returned ID to get_jobs() will silently fail. The format is also completely different (integer vs ULID).

Yes! but i think we should do an internal mapping from Ulid to TaskId in case we have a usecase for it in the future like with a Map (or smth similar)

Previously, if BackgroundQueue mode was configured but no provider was set, it logged an error and continued. Now it returns Err(...). This will crash apps that previously worked in a degraded state. Could we keep this as a warning/error log, or at minimum call this out as an intentional behavior change?

i think so yes!

it is on my todo board!

@NewtTheWolf
Copy link
Contributor Author

Hey @kaplanelad, I addressed your points:

Inconsistent ID semantics:
BackgroundAsync now generates a ULID instead of returning the tokio task::Id. All three modes return consistent
ULID-formatted IDs now. I also added an AsyncJobTaskMap that internally maps the ULID to the tokio task ID, so we
retain access to it if needed in the future. The map is accessible via the SharedStore.

Error on missing provider:
Reverted back to tracing::error!() with a fallback ULID instead of returning Err(...), so apps in a degraded state
won't crash.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants